Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(term): add descriptive error on Windows for lack of VT #32

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

Tieske
Copy link
Member

@Tieske Tieske commented Jul 21, 2024

if no virtual terminal processing is available, raise a descriptive error instead of a failure on some missing defines.

closes #29

@tobil4sk
Copy link

tobil4sk commented Jul 26, 2024

After further investigation, it looks like the error in #29 is unrelated to Visual Studio 2019. All windows SDKs that are available to install with the Visual Studio 2019 installer have this defined. The main way of running into this issue seems to be when using an old mingw build, in particular, a build with a mingw runtime older than v7.

@Tieske
Copy link
Member Author

Tieske commented Jul 28, 2024

@tobil4sk so the check is still relevant, but the message must be changed:

#error 2019 and older toolchains are not supported. Update the toolchain or revert to Luasystem < 0.4

should become

#error Virtual terminal macros are undefined (eg. ENABLE_VIRTUAL_TERMINAL_INPUT). Update the toolchain or revert to Luasystem < 0.4

correct?

@tobil4sk
Copy link

correct?

Yes, that works. I think the most likely way to run into this is by using an old version of mingw (people might install it because the latest build that shows up on sourceforge is very old), but a generic message like this works too.

@Tieske Tieske force-pushed the vtperror branch 2 times, most recently from 6c3ac22 to da7c280 Compare July 30, 2024 11:05
@Tieske
Copy link
Member Author

Tieske commented Jul 30, 2024

@o-lim I think this is good to go now. Please have a look.

src/term.c Outdated
@@ -107,6 +107,11 @@ typedef struct ls_RegConst {
// This is needed because some flags are not defined on all platforms. So we
// still export the constants, but they will be all 0, and hence not do anything.
#ifdef _WIN32
// check compatibility: Windows virtual terminal processing was added in 2019,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added before 2019, so maybe it's best to change this comment too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are correct, thx. New Windows Terminal app was added in 2019, but virtual processing was added in 2015. I'll update accordingly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobil4sk please check the updated comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks

if no virtual terminal processing is available, raise a descriptive
error instead of a failure on some missing defines.
@Tieske Tieske merged commit 227743a into master Jul 31, 2024
23 checks passed
@Tieske Tieske deleted the vtperror branch July 31, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Error when install: Error: Build error: Failed compiling object src/term.o
3 participants